Repository navigation
Audit extension settings and item JSON updates; repair Bun merge drivers - #1348
Conversation
Expose a host-bound settings mutation primitive to extension commands. Apply validated changes under the workspace-history lock, support dry-run and idempotent retries, and attribute events to the invoking actor. Reject full-item JSON updates that modify persisted comments, notes, or learnings. Check the submitted snapshot under the item lock so annotation-only edits cannot return a successful no-op. Install and audit an absolute Bun merge-driver launcher alongside Node. Cover packed Bun and copied-extension workflows, record the three issue resolutions in pm, and regenerate the SDK contracts and changelog.
Reviewer's GuideThis PR adds an audited, replay-safe host SDK mutation for project settings, enforces annotation integrity for full-item JSON updates, repairs Bun merge-driver installation and validation, and updates the associated contracts, tests, documentation, and PM/changelog artifacts. Sequence diagram for audited extension settings mutationsequenceDiagram
participant Extension as ExtensionCommand
participant SDK as HostBoundSDK
participant History as WorkspaceHistory
participant Settings as settings.json
Extension->>SDK: mutateWorkspaceSettings(operationId, mutate, dryRun)
SDK->>History: mutateWorkspaceJsonWithHistory(operationId, author, dryRun)
History->>History: acquireLock(workspace-history)
History->>History: verifyHistoryChain()
History->>SDK: locked current settings
SDK->>SDK: validateSettings(current)
SDK->>Extension: mutate(current)
Extension-->>SDK: next settings
SDK->>SDK: validateSettings(next)
alt dryRun
SDK-->>Extension: changed, dry_run, replayed
else new operation
History->>Settings: write validated settings
History->>History: append audited history event
History-->>SDK: mutation result
SDK-->>Extension: changed, dry_run, replayed
else recorded operation
History-->>SDK: replayed result
SDK-->>Extension: changed: false, replayed: true
end
Flow diagram for full-item JSON annotation integrityflowchart TD
A[Full-item --stdin-json update] --> B[itemDocumentToMutationOptions]
B --> C[documentAnnotationSnapshot]
C --> D[mutateUpdateDocument]
D --> E[assertPersistedAnnotationSnapshot]
E -->|Existing comments, notes, or learnings changed| F[Reject with stdin_json_persisted_annotation_changed]
E -->|Persisted annotations unchanged| G[Append new annotations and write item fields]
F --> H[Use pm comments, notes, or learnings --edit]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/core/history/workspace-history.ts" line_range="539-542" />
<code_context>
+ { code: "workspace_history_state_conflict" },
+ );
+ }
+ if (params.idempotencyKey !== undefined && entries.some(
+ (entry) => entry.op === `${params.op}:${params.idempotencyKey}`,
+ )) {
+ return { changed: false, result: (await params.mutate(beforeRaw)).result, replayed: true };
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** A replayed idempotent mutation invokes `params.mutate(beforeRaw)` again before returning instead of returning the recorded result without executing the mutation callback. Retried extension commands therefore rerun arbitrary callback side effects and can fail or perform expensive work even though no settings or history write occurs.
**Triggers:** When an extension retries an operation ID that already has a history entry.
**Suggested fix:** Detect the replay before invoking the callback and return a stored or otherwise replay-safe result without executing the mutation function.
</issue_to_address>
### Comment 2
<location path="src/sdk/structured-mutations.ts" line_range="848-855" />
<code_context>
appendFacetOptions(documentOptions, item, mode === "update");
if (linked !== undefined) appendFacetOptions(documentOptions, linked);
+ if (mode === "update") {
+ const annotations = Object.fromEntries(
+ (["comments", "notes", "learnings"] as const)
+ .filter((key) => Object.hasOwn(item, key))
+ .map((key) => [
+ key,
+ (item[key] as unknown[]).filter(
+ (entry) => isPlainObject(entry) && entry.created_at !== undefined,
+ ),
+ ]),
+ );
</code_context>
<issue_to_address>
**issue (bug_risk):** The full-item JSON conversion calls `.filter` on each supplied `comments`, `notes`, or `learnings` value without first checking that it is an array. A malformed but parseable document such as `{"notes": null}` therefore raises a raw TypeError instead of producing the command's normal usage validation error.
**Triggers:** When `--stdin-json` contains a non-array value for one of the annotation fields.
**Suggested fix:** Validate each annotation field is an array before filtering, and raise the appropriate `PmCliError` for invalid full-item JSON.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and this changes several persisted mutation paths: an extension can write settings and workspace history, full-item updates now reject annotation edits, and Bun merge-driver configuration affects local Git merges. A wrong settings or merge result can outlive a code revert, but the affected files and history are bounded and can be repaired or regenerated.
Blocking findings: src/core/history/workspace-history.ts:542, src/sdk/structured-mutations.ts:855
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Keep replay callbacks inert, scope extension operation IDs to their command, serialize settings through the canonical writer, and notify write hooks for changed state. Record the public SDK receipt contract and regression acceptance for copied extensions and malformed annotation input. Complete the linked PM defect gate evidence, exact coverage and static verification, and refreshed reviewed documentation.
|
@greptileai Please review the updated head, including the replay, command identity, serializer, and hook fixes. |
❌ Action failedReview failed. |
|
Reviewed Sourcery guide #1348 (comment). The replay finding was fixed and the annotation validation path was proven with regression tests; both inline threads are resolved. |
|
Reviewed CodeRabbit automatic review notice #1348 (comment). A manual full review was requested for head 78bd206; the provider returned a failed invocation without findings. Required checks and other reviews completed. |
|
Reviewed Greptile review #1348 (review). Its four inline findings were addressed in 78bd206, each thread has a response, and the updated summary reports no remaining issue. |
|
Reviewed Codecov report #1348 (comment). The hosted patch check passed; local exact V8 statements, branches, functions, and lines are all 100 percent. |
|
Reviewed CodeRabbit manual invocation response #1348 (comment). The provider reported review failure without actionable findings after the explicit full-review request. Required checks and the Greptile and Sourcery review surfaces are complete. |
|
Reviewed Sourcery review #1348 (review). Callback replay is fixed; the malformed annotation claim is refuted by the existing typed validator and new regression assertions. Both findings have inline dispositions. |
|
Reviewed CodSpeed report #1348 (comment). It reports 11 untouched benchmarks on the current head; no performance change is requested. |
|
Reviewed updated Greptile summary #1348 (comment). Its four original findings are fixed and resolved in their inline threads; the exact-head summary reports safe to merge. |
Summary
PM items
Review follow-up
onWritehooks after a changed write. Installed-extension tests cover both behaviors.Verification
pnpm quality:static,pnpm typecheck, linkedpm test --run --progress, changelog--check, andpnpm security:scanpassed.npxandbunxboth report2026.9.29; Bun strict health returned zero findings. Separate temporary Git merge acceptance exercised the Bun item/history drivers and verified history.pm validate --check-resolution --check-history-driftfound zero drift across 2,839 items. Its sole warning is the previously documented 170-item historical resolution evidence residual owned by pm-e9zh.